Syncs submodules after remote update (closes #370)

Previously, the Git code avoided cloning a local copy of the local
database for a repository if the repo already existed (and instead tried
to fetch). However, fetching does not sync and reinitialize submodules,
which would require more work.

To avoid this problem, and possibly other subtle updating issues in the
future, the code now wipes the local clone whenever it detects that the
HEAD of a particular branch has changed. This avoids subtle
inconsistencies between fresh clones and updates.

Note: This only affects local-to-local clones. It does not initiate any
new network activity. It may require some additional disk usage, but
only when a remote fetch that was already happening discovers new
commits on the checked-out branch.

diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs
index fec0b23edd19501345c5d630aee85c3fed3dcdad..61577b50c01f88e84fe5105648551536b0732782 100644 (file)
--- a/src/cargo/sources/git/utils.rs
+++ b/src/cargo/sources/git/utils.rs
@@ -206,17 +206,17 @@ impl GitDatabase {
 
     pub fn copy_to(&self, rev: GitRevision, dest: &Path)
                    -> CargoResult<GitCheckout> {
-        let checkout = try!(GitCheckout::clone_into(dest, self.clone(),
-                                                    rev.clone()));
 
-        match self.remote.rev_for(dest, "HEAD") {
-            Ok(ref head) if rev == *head => {}
-            _ => try!(checkout.fetch()),
+        if dest.exists() {
+            match self.remote.rev_for(dest, "HEAD") {
+                Ok(ref head) if rev == *head => {
+                    return Ok(GitCheckout::new(dest, self.clone(), rev.clone()));
+                }
+                _ => {}
+            }
         }
 
-        try!(checkout.update_submodules());
-
-        Ok(checkout)
+        GitCheckout::clone_into(dest, self.clone(), rev.clone())
     }
 
     pub fn rev_for<S: Str>(&self, reference: S) -> CargoResult<GitRevision> {
@@ -230,18 +230,24 @@ impl GitDatabase {
 }
 
 impl GitCheckout {
-    fn clone_into(into: &Path, database: GitDatabase,
-                  revision: GitRevision) -> CargoResult<GitCheckout> {
-        let checkout = GitCheckout {
-            location: into.clone(),
+    fn new(path: &Path, database: GitDatabase, revision: GitRevision)
+           -> GitCheckout
+    {
+        GitCheckout {
+            location: path.clone(),
             database: database,
             revision: revision,
-        };
-
-        // If the git checkout already exists, we don't need to clone it again
-        if !checkout.location.join(".git").exists() {
-            try!(checkout.clone_repo());
         }
+    }
+
+    fn clone_into(into: &Path, database: GitDatabase,
+                  revision: GitRevision)
+                  -> CargoResult<GitCheckout>
+    {
+        let checkout = GitCheckout::new(into, database, revision);
+
+        try!(checkout.clone_repo());
+        try!(checkout.update_submodules());
 
         Ok(checkout)
     }
@@ -276,38 +282,13 @@ impl GitCheckout {
         Ok(())
     }
 
-    fn fetch(&self) -> CargoResult<()> {
-        // In git 1.8, apparently --tags explicitly *only* fetches tags, it does
-        // not fetch anything else. In git 1.9, however, git apparently fetches
-        // everything when --tags is passed.
-        //
-        // This means that if we want to fetch everything we need to execute
-        // both with and without --tags on 1.8 (apparently), and only with
-        // --tags on 1.9. For simplicity, we execute with and without --tags for
-        // all gits.
-        //
-        // FIXME: This is suspicious. I have been informed that, for example,
-        //        bundler does not do this, yet bundler appears to work!
-        //
-        // And to continue the fun, git before 1.7.3 had the fun bug that if a
-        // branch was tracking a remote, then `git fetch $url` doesn't work!
-        //
-        // For details, see
-        // https://www.kernel.org/pub/software/scm/git/docs/RelNotes-1.7.3.txt
-        //
-        // In this case we just use `origin` here instead of the database path.
-        git!(self.location, "fetch", "--force", "--quiet", "origin");
-        git!(self.location, "fetch", "--force", "--quiet", "--tags", "origin");
-        try!(self.reset());
-        Ok(())
-    }
-
     fn reset(&self) -> CargoResult<()> {
         Ok(git!(self.location, "reset", "-q", "--hard",
                 self.revision.as_slice()))
     }
 
     fn update_submodules(&self) -> CargoResult<()> {
+        git!(self.location, "submodule", "sync", "--quiet");
         Ok(git!(self.location, "submodule", "update", "--init",
                 "--recursive", "--quiet"))
     }

diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
index 5d0d8a6c52183c6e4ea5fa9bf4ea92cddaa913f2..5e337b37e34d15123a25b398fbaf35ff59d22343 100644 (file)
--- a/tests/test_cargo_compile_git_deps.rs
+++ b/tests/test_cargo_compile_git_deps.rs
@@ -10,6 +10,7 @@ use hamcrest::{assert_that,existing_file};
 use cargo;
 use cargo::util::{ProcessError, process};
 
+
 fn setup() {
 }